Codecov Report

❌ Patch coverage is 77.14286% with 64 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!

/review

/simplify

codeagent is processing your request, please wait a moment...

@xgopilot 给我一个能给AI用来review的提示词，先确认当前总体方案设计，再确认有没有存在风险点（诸如方案设计、模块边界等等）

可直接使用的 Review 提示词

你可以把下面整段直接贴给 AI（建议配合当前 PR 的 diff 一起输入）：

你是一个严格的软件架构与代码审查专家。请对当前 PR 做“先方案、后风险”的审查。

【审查目标】
1) 先确认总体方案设计是否成立（设计目标、主链路闭环、模块职责、跨层依赖、演进方向）。
2) 再识别风险点（方案设计风险、模块边界风险、并发/一致性风险、性能退化风险、安全风险、可测试性风险、回归风险）。

【仓库关键约束（必须用于判定）】
- 主链路必须可用：用户输入 -> Agent 推理 -> 调用工具 -> 获取结果 -> 继续推理 -> UI 展示。
- 分层边界：TUI -> Runtime -> Provider/Tool Manager，不允许跨层直连。
- Provider 差异只能收敛在 `internal/provider`，不能泄漏到 runtime/tui。
- 工具能力必须在 `internal/tools`，runtime/tui 不直接执行工具逻辑。
- 会话状态与消息历史应由 runtime 管理，不散落到 UI。
- 配置/安全：不能明文密钥；关键配置需校验；工具要有越界/超时/输出限制。
- 测试目标：新增或变更逻辑要覆盖正常、边界、异常、回归分支。

【输入信息】
- PR 标题：perf(session,memo,context): 三模块全量性能优化与安全加固
- 重点模块：session / memo / context / runtime
- 声称改动：过期清理、消息裁剪、权限收紧、索引缓存、指纹去重、git 超时、RWMutex 等

【输出格式（严格遵守）】
A. 总体方案评估（先给结论）
- 结论：通过 / 有条件通过 / 不通过
- 设计一致性：是否符合主链路与分层边界
- 关键证据：引用具体文件与行号（`path:line`）

B. 风险清单（按严重级别排序）
- 每条风险必须包含：
  - 标题
  - 严重级别（Critical/High/Medium/Low）
  - 证据（文件+行号+触发条件）
  - 影响面（功能/性能/安全/可维护性）
  - 修复建议（最小可行改动）
  - 需要补充的测试点

C. 边界与职责专项检查
- 是否存在跨层调用、职责下沉/上浮、厂商字段泄漏、状态归属错误
- 若存在，给出重构落点（应该放在哪个模块）

D. 回归与发布建议
- 必测回归用例列表
- 发布阻断项（若有）

【审查要求】
- 不要泛泛而谈，只输出有证据的问题。
- 没有证据的推测必须显式标注“待确认”。
- 若未发现问题，明确写“未发现阻断问题”，并给出剩余观察风险。

说明

已按你的要求给出“先确认总体方案，再确认风险点（含模块边界）”的可执行提示词。本次仅提供提示词模板，未直接执行代码审查。

View job run

@xgopilot 你是一个严格的软件架构与代码审查专家。请对当前 PR 做“先方案、后风险”的审查。

【审查目标】

1. 先确认总体方案设计是否成立（设计目标、主链路闭环、模块职责、跨层依赖、演进方向）。
2. 再识别风险点（方案设计风险、模块边界风险、并发/一致性风险、性能退化风险、安全风险、可测试性风险、回归风险）。

【仓库关键约束（必须用于判定）】

• 主链路必须可用：用户输入 -> Agent 推理 -> 调用工具 -> 获取结果 -> 继续推理 -> UI 展示。
• 分层边界：TUI -> Runtime -> Provider/Tool Manager，不允许跨层直连。
• Provider 差异只能收敛在 internal/provider，不能泄漏到 runtime/tui。
• 工具能力必须在 internal/tools，runtime/tui 不直接执行工具逻辑。
• 会话状态与消息历史应由 runtime 管理，不散落到 UI。
• 配置/安全：不能明文密钥；关键配置需校验；工具要有越界/超时/输出限制。
• 测试目标：新增或变更逻辑要覆盖正常、边界、异常、回归分支。

【输入信息】

• PR 标题：perf(session,memo,context): 三模块全量性能优化与安全加固
• 重点模块：session / memo / context / runtime
• 声称改动：过期清理、消息裁剪、权限收紧、索引缓存、指纹去重、git 超时、RWMutex 等

【输出格式（严格遵守）】
A. 总体方案评估（先给结论）

• 结论：通过 / 有条件通过 / 不通过
• 设计一致性：是否符合主链路与分层边界
• 关键证据：引用具体文件与行号（path:line）

B. 风险清单（按严重级别排序）

• 每条风险必须包含：
  • 标题
  • 严重级别（Critical/High/Medium/Low）
  • 证据（文件+行号+触发条件）
  • 影响面（功能/性能/安全/可维护性）
  • 修复建议（最小可行改动）
  • 需要补充的测试点

C. 边界与职责专项检查

• 是否存在跨层调用、职责下沉/上浮、厂商字段泄漏、状态归属错误
• 若存在，给出重构落点（应该放在哪个模块）

D. 回归与发布建议

• 必测回归用例列表
• 发布阻断项（若有）

【审查要求】

• 不要泛泛而谈，只输出有证据的问题。
• 没有证据的推测必须显式标注“待确认”。
• 若未发现问题，明确写“未发现阻断问题”，并给出剩余观察风险。

A. 总体方案评估（先给结论）

• 结论：有条件通过
• 设计一致性：本 PR 的改动主要落在 session/memo/context/runtime/app 内部实现，未发现 tui -> runtime -> provider/tools 主链路被跨层打穿；provider 厂商差异也未扩散到 runtime/tui。
• 关键证据：
  • 启动期清理接入在 app 装配层：internal/app/bootstrap.go:160
  • 运行时系统工具仍通过 runtime + tool manager：internal/runtime/system_tool.go:76
  • provider 相关改动未进入本 PR 核心逻辑路径（仅 runtime 内部锁优化）：internal/runtime/session_scheduler.go:110
  • context git 超时收敛在 context 模块：internal/context/source_system.go:111

B. 风险清单（按严重级别排序）

• 标题：过期清理删除附件目录时缺少路径越界防护

• 严重级别：High

• 证据（文件+行号+触发条件）：internal/session/sqlite_store.go:126-129 直接 os.RemoveAll(filepath.Join(s.assetsDir, id))；触发条件是 DB 内存在异常/篡改的 id（绕过 API 校验）。对比已有安全做法：internal/session/sqlite_store.go:669-671 在删除单会话资产前会做 ensurePathWithinBase。

• 影响面：安全 / 数据完整性（潜在越界删除）。

• 修复建议（最小可行改动）：在清理循环中复用 ensurePathWithinBase（或先 validateStorageID 再 ensurePathWithinBase），校验失败时跳过并记录告警。

• 需要补充的测试点：构造“恶意 session id（通过直接写库）+ cleanup”场景，断言工作目录外文件不被删除。

• 标题：权限“收紧”对已存在目录不生效

• 严重级别：Medium

• 证据（文件+行号+触发条件）：internal/session/sqlite_store.go:693-698 仅 MkdirAll(..., 0o700)；已存在目录时不会修改其 mode。触发条件：历史安装目录此前为 0755。

• 影响面：安全 / 合规（S-5 目标与实际不一致）。

• 修复建议（最小可行改动）：MkdirAll 后补 os.Chmod(s.projectDir, 0o700)、os.Chmod(s.assetsDir, 0o700)（Unix 生效，Windows 可按平台分支处理）。

• 需要补充的测试点：预建 0755 目录后初始化 store，断言 mode 被收紧（Unix）。

• 标题：增量指纹未覆盖非文本消息，可能误判“未变化”

• 严重级别：Medium

• 证据（文件+行号+触发条件）：internal/memo/auto_extractor.go:324-334 仅哈希 Role + Text parts。触发条件：文本不变但图片/其他非文本内容变化。

• 影响面：功能正确性（会跳过应执行的自动提取）。

• 修复建议（最小可行改动）：指纹至少纳入 part.Kind 及非文本关键字段（建议对 message 做稳定序列化后哈希，避免字段遗漏）。

• 需要补充的测试点：两次调度文本相同但图片/非文本 part 不同，应触发第二次提取。

• 标题：启动清理入口退化为具体实现耦合，削弱 Store 抽象

• 严重级别：Low

• 证据（文件+行号+触发条件）：internal/app/bootstrap.go:38-44 的 cleanupExpiredSessions 显式依赖 *agentsession.SQLiteStore，而接口已在 internal/session/store.go:127-128 定义 CleanupExpiredSessions。

• 影响面：可维护性 / 演进性（未来切换 store 实现时，app 层需改代码）。

• 修复建议（最小可行改动）：将 cleanupExpiredSessions 及调用面收敛到 agentsession.Store 接口。

• 需要补充的测试点：使用 Store mock/stub 验证 BuildRuntime 启动清理分支，而不依赖 sqlite 具体类型。

C. 边界与职责专项检查

• 跨层调用：未发现 tui 直连 provider/tool 执行。
• 职责下沉/上浮：未发现 runtime/tui 直接承载 tools 执行逻辑。
• 厂商字段泄漏：未发现 provider 厂商差异泄漏到 runtime/tui。
• 状态归属：会话状态仍由 runtime/session 管理，未散落到 UI。
• 待优化点：app 启动清理对具体类型 *SQLiteStore 的耦合（见 internal/app/bootstrap.go:38-44），建议回归 Store 抽象。

D. 回归与发布建议

• 必测回归用例列表：

  1. 启动清理：过期/未过期会话混合，验证仅删除过期会话及其附件。
  2. 安全防护：异常 session id（直写 DB）触发 cleanup，不得越界删除。
  3. 权限收紧：已存在 projectDir/assetsDir 的 mode 从 0755 收敛到 0700（Unix）。
  4. Append 裁剪：MaxSessionMessages 边界（=8192、>8192、大批次追加）。
  5. AutoExtractor 指纹：文本相同但非文本变化必须触发提取。
  6. Context git 超时：模拟阻塞命令，确认 5s 超时后不中断主链路。

• 发布阻断项：有

  • High：cleanup 附件删除缺少路径越界校验（建议修复后再合入）。

补充：本地已执行 go test ./internal/session ./internal/memo ./internal/context ./internal/runtime ./internal/app，均通过。

View job run

@xgopilot 现在修复过一轮review了，请你再进行一轮审查式review

审查结果（问题项）

1. 按字节裁剪路径未遵循“按优先级淘汰”策略（Medium）

• 证据：internal/memo/service.go:503-505 在 maxIndexBytes 超限时调用 trimIndexEntriesByBytes；internal/memo/service.go:535-549 通过 entries[lo:] 仅按“最旧优先”删除前缀条目。
• 触发条件：maxEntries 未触发或触发后仍超 maxIndexBytes。
• 影响：会把高优先级但更旧的记忆先删掉，和 PR 声称的 user > feedback > project > reference（且 manual 优先）策略不一致。
• 最小修复建议：字节裁剪阶段也按 evictionPriority() 选 victim（循环移除直到 RenderIndex(index) <= maxIndexBytes），不要仅按时间前缀裁剪。
• 建议补测：构造“旧的高优先级 + 新的低优先级”并触发 maxIndexBytes，断言删除低优先级条目。

2. 过期会话附件清理不响应 context 取消（Low）

• 证据：internal/session/sqlite_store.go:127-129 在事务提交后直接调用 cleanupExpiredSessionAssets(expiredIDs)；internal/session/sqlite_store.go:1226-1232 内部循环 os.RemoveAll，无 ctx.Err() 检查。
• 触发条件：过期会话/附件规模较大、慢磁盘或网络挂载场景下，外部已取消上下文。
• 影响：启动阶段取消不及时，可能额外阻塞启动/退出路径。
• 最小修复建议：将 ctx 传入 cleanupExpiredSessionAssets，每轮删除前检查 ctx.Err() 并中止。
• 建议补测：构造多个待清理目录，在清理中途取消 context，断言函数尽快返回并停止后续删除。

说明

• 本轮仅执行审查与验证，未改动代码。
• 已本地执行：go test ./internal/session ./internal/memo ./internal/context ./internal/runtime ./internal/app。

View job run